pyproject.toml-3-3 (1)

3-3: Version mismatch between pyproject.toml and init.py.

The version here is 0.1.14b0 (beta), but SCR/valetudo_map_parser/__init__.py declares Version: 0.1.14 (without the beta suffix). Consider aligning both version declarations to avoid confusion.

tests/VALETUDO_MAP_PARSER_TYPES_USAGE_REPORT.md-2-2 (1)

2-2: Incorrect generation date.

The document states it was generated on 2025-10-18, but the PR was created on 2025-12-20. Update the date to reflect when this was actually generated.

🔎 Proposed fix

 # Valetudo Map Parser Types Usage Report
-**Generated:** 2025-10-18  
+**Generated:** 2025-12-20  
 **Purpose:** Comprehensive analysis of all valetudo_map_parser types, classes, and constants currently in use

tests/convert_rand_to_hypfer.py-309-312 (1)

309-312: Potential division by zero.

If compressed_pixels is 0, lines 311-312 will raise a ZeroDivisionError.

🔎 Proposed fix

     print(f"Original pixel data: {original_pixels} values")
     print(f"Compressed pixel data: {compressed_pixels} values")
-    print(f"Compression ratio: {original_pixels / compressed_pixels:.2f}x")
-    print(f"Memory reduction: {(1 - compressed_pixels/original_pixels) * 100:.1f}%")
+    if compressed_pixels > 0 and original_pixels > 0:
+        print(f"Compression ratio: {original_pixels / compressed_pixels:.2f}x")
+        print(f"Memory reduction: {(1 - compressed_pixels/original_pixels) * 100:.1f}%")
+    else:
+        print("Unable to calculate compression ratio (no pixel data)")

backups/rand25_handler_rooms.py-180-180 (1)

180-180: Invalid return type annotation syntax.

PilPNG or None is not valid Python type syntax. Use Optional[PilPNG] or PilPNG | None (Python 3.10+).

🔎 Proposed fix

+from typing import Any, Optional
+
 async def get_image_from_rrm(
     self,
     m_json: JsonType,  # json data
     destinations: None = None,  # MQTT destinations for labels
-) -> PilPNG or None:
+) -> Optional[PilPNG]:

Committable suggestion skipped: line range outside the PR's diff.

backups/colors.py-456-456 (1)

456-456: Return type mismatch: Returns list instead of tuple.

The method signature and docstring indicate returning a Color (tuple), but line 456 returns a list [out_r, out_g, out_b, out_a]. This inconsistency may cause issues if downstream code expects tuple operations (e.g., unpacking, hashing).

🔎 Suggested fix

-        return [out_r, out_g, out_b, out_a]
+        return (out_r, out_g, out_b, out_a)

backups/drawable_ori.py-34-40 (1)

34-40: Minor: Comment does not match the constant value.

The comment on line 34 says "Red color for error messages" but ERROR_OUTLINE is set to black (0, 0, 0, 255). Similarly, line 40's comment says "Red color with lower opacity for error outlines" but the constant name is ERROR_COLOR. The naming and comments are swapped.

🔎 Suggested fix

-    ERROR_OUTLINE: Color = (0, 0, 0, 255)  # Red color for error messages
-    ERROR_COLOR: Color = (
-        255,
-        0,
-        0,
-        191,
-    )  # Red color with lower opacity for error outlines
+    ERROR_OUTLINE: Color = (0, 0, 0, 255)  # Black color for error outlines
+    ERROR_COLOR: Color = (
+        255,
+        0,
+        0,
+        191,
+    )  # Red color with lower opacity for error fill

tests/compare_payloads.py-12-12 (1)

12-12: Fix import module name - typo in module reference.

Import references rand25_parser but the actual module is rand256_parser. This will cause an ImportError at runtime.

-from valetudo_map_parser.config.rand25_parser import RRMapParser
+from valetudo_map_parser.config.rand256_parser import RRMapParser

tests/test_mvcrender.py-71-74 (1)

71-74: Missing f-string prefix causes incorrect output.

Line 73 is missing the f prefix, so it will print the literal string {ITERATIONS} instead of the value 100.

🔎 Proposed fix

 # Test 1: AutoCrop with rotation (most complex operation)
 print(f"\n{'=' * 70}")
-print("Test 1: AutoCrop with rotation ({ITERATIONS} iterations)")
+print(f"Test 1: AutoCrop with rotation ({ITERATIONS} iterations)")
 print(f"{'=' * 70}")

tests/rooms_test.py-119-125 (1)

119-125: os.makedirs with empty string path.

os.path.dirname("test.json") returns an empty string since the file has no directory component. os.makedirs("", exist_ok=True) will either fail or be a no-op depending on the OS. This logic should be removed or handle the edge case.

🔎 Proposed fix

-        os.makedirs(os.path.dirname(test_data_path), exist_ok=True)
+        dir_name = os.path.dirname(test_data_path)
+        if dir_name:  # Only create directory if there's a directory component
+            os.makedirs(dir_name, exist_ok=True)
         with open(test_data_path, "w", encoding="utf-8") as file:
             json.dump(sample_data, file, indent=2)

new_tests/integration/test_basic_integration.py-52-65 (1)

52-65: Missing image cleanup in test.

Similar to the calibration points test, the image from line 59 should be closed.

🔎 Proposed fix

     @pytest.mark.asyncio
     async def test_hypfer_room_detection(self, hypfer_json_data, vacuum_id, device_info):
         """Test that rooms are detected from JSON."""
         manager = CameraSharedManager(vacuum_id, device_info)
         shared = manager.get_instance()
         handler = HypferMapImageHandler(shared)

-        await handler.async_get_image(hypfer_json_data)
+        image, _ = await handler.async_get_image(hypfer_json_data)

         # Check if rooms were detected
         room_store = RoomStore(vacuum_id)
         rooms = room_store.get_rooms()
         # Should have detected some rooms (depends on test data)
         assert isinstance(rooms, dict)
+
+        # Clean up
+        if image:
+            image.close()

new_tests/integration/test_basic_integration.py-39-50 (1)

39-50: Missing image cleanup in test.

The image generated on line 46 is not closed, which could lead to resource leaks during test runs.

🔎 Proposed fix

     @pytest.mark.asyncio
     async def test_hypfer_calibration_points(self, hypfer_json_data, vacuum_id, device_info):
         """Test that calibration points are generated."""
         manager = CameraSharedManager(vacuum_id, device_info)
         shared = manager.get_instance()
         handler = HypferMapImageHandler(shared)

         image, metadata = await handler.async_get_image(hypfer_json_data)

         # Check calibration points were set (may be None if image generation had errors)
         # This is acceptable as the library may have issues with certain data
         assert shared.attr_calibration_points is None or isinstance(shared.attr_calibration_points, list)
+
+        # Clean up
+        if image:
+            image.close()

tests/test_parser_comparison.py-221-230 (1)

221-230: Inconsistent pixel flag usage between test calls.

Line 224 calls compare_parsers which internally sets pixels=True, but the comment on line 223 says "basic parsing (pixels=False)". Then line 229 calls test_with_pixels which also uses pixels=True. Both tests are effectively the same.

🔎 Proposed fix

Update compare_parsers to use pixels=False for basic parsing:

 def compare_parsers(payload_file: str, runs: int = 5):
     ...
     # Profile both parsers
     rand25_stats = profile_parser(
-        Rand25Parser(), "RAND25", payload, pixels=True, runs=runs
+        Rand25Parser(), "RAND25", payload, pixels=False, runs=runs
     )
     rand256_stats = profile_parser(
-        Rand256Parser(), "RAND256", payload, pixels=True, runs=runs
+        Rand256Parser(), "RAND256", payload, pixels=False, runs=runs
     )

Committable suggestion skipped: line range outside the PR's diff.

new_tests/integration/test_basic_integration.py-94-106 (1)

94-106: Missing image cleanup in Rand256 calibration test.

Same issue as Hypfer tests - the image should be closed after use.

🔎 Proposed fix

         # Check calibration points were set (may be None if image generation had errors)
         # This is acceptable as the library may have issues with certain data
         assert shared.attr_calibration_points is None or isinstance(shared.attr_calibration_points, list)
+
+        # Clean up
+        if image:
+            image.close()

backups/hypfer_rooms_handler.py-21-22 (1)

21-22: Same typo in class docstring.

🔎 Suggested fix

-    Handler for extracting and managing room data from Hipfer vacuum maps.
+    Handler for extracting and managing room data from Hypfer vacuum maps.

backups/hypfer_rooms_handler.py-31-40 (1)

31-40: Typo in method docstring: "HipferRoomsHandler" should be "HypferRoomsHandler".

🔎 Suggested fix

-        Initialize the HipferRoomsHandler.
+        Initialize the HypferRoomsHandler.

backups/hypfer_rooms_handler.py-2-6 (1)

2-6: Typo: "Hipfer" should be "Hypfer"

The module docstring and class docstrings consistently misspell "Hypfer" as "Hipfer".

🔎 Suggested fix

 """
-Hipfer Rooms Handler Module.
-Handles room data extraction and processing for Valetudo Hipfer vacuum maps.
+Hypfer Rooms Handler Module.
+Handles room data extraction and processing for Valetudo Hypfer vacuum maps.
 Provides async methods for room outline extraction and properties management.
 Version: 0.1.9
 """

backups/hypfer_handler_ori.py-97-109 (1)

97-109: Type hint uses json module as type annotation.

Line 100 uses m_json: json | None which refers to the json module, not a JSON data type. This should be dict | None or a more specific type like Dict[str, Any].

🔎 Suggested fix

     async def async_get_image_from_json(
         self,
-        m_json: json | None,
+        m_json: dict | None,
         return_webp: bool = False,
     ) -> WebPBytes | Image.Image | None:

backups/drawable.ori-34-40 (1)

34-40: Misleading comment: both colors are red-based.

The comment for ERROR_OUTLINE says "Red color" but the value is (0, 0, 0, 255) which is black, not red.

🔎 Suggested fix

-    ERROR_OUTLINE: Color = (0, 0, 0, 255)  # Red color for error messages
+    ERROR_OUTLINE: Color = (0, 0, 0, 255)  # Black outline for error states
     ERROR_COLOR: Color = (
         255,
         0,
         0,
         191,
-    )  # Red color with lower opacity for error outlines
+    )  # Red fill color with transparency for error states

backups/map_data_ori.py-451-459 (1)

451-459: Invalid return type annotation syntax.

tuple or list is not valid Python type syntax. Use Union[tuple, list] or the proper return type.

🔎 Suggested fix

     @staticmethod
     async def async_get_rrm_segments(
         json_data: JsonType,
         size_x: int,
         size_y: int,
         pos_top: int,
         pos_left: int,
         out_lines: bool = False,
-    ) -> tuple or list:
+    ) -> Union[Tuple[list, list], list]:

Committable suggestion skipped: line range outside the PR's diff.

new_tests/config/__init__.py (1)

1-1: Minor typo: double period in docstring.

The docstring has an extra period at the end ("module..").

Suggested fix

-"""Tests for config module."""
+"""Tests for config module."""

Note: If the double period was intentional, disregard this comment.

tests/debug_manage_rooms.py (1)

13-16: Consider using specific exception type instead of bare except.

The bare except: clause catches all exceptions, including SystemExit and KeyboardInterrupt, which may interfere with debugging and interrupt handling.

🔎 Suggested improvement

 try:
     asyncio.run(main())
-except Exception:
+except Exception:
     traceback.print_exc(file=sys.stdout)

Note: The current code already uses Exception, so this is already correct. No changes needed.

backups/refactored_old_code.py (1)

1-44: Consider removing commented-out code and rely on version control.

This entire file contains only commented-out code. Version control systems like Git are designed to preserve code history, making it unnecessary to keep large blocks of commented code in the repository.

If this code needs to be preserved for reference:

• Consider documenting the approach in a markdown file with a reference to the relevant commit
• Or simply rely on Git history to access the old implementation

new_tests/README.md (1)

7-21: Consider adding language specifier to fenced code block.

The directory structure visualization uses a fenced code block without a language identifier. While not critical, adding a language specifier improves rendering and syntax highlighting.

🔎 Suggested improvement

-```
+```text
 new_tests/
 ├── conftest.py                          # Pytest fixtures and configuration
 ├── config/                              # Tests for config module

Based on static analysis hints.

tests/analyze_room12.py (2)

79-79: Move scipy import to module level.

The scipy import is placed inside the function, which is unconventional. Move it to the top of the file with other imports for better clarity and to catch import errors early.

🔎 Proposed fix

 import numpy as np
+from scipy import ndimage
 

 
 # Set up logging

         _LOGGER.info(
             f"Scaled bounding box: X: {x_min * pixel_size}-{x_max * pixel_size}, Y: {y_min * pixel_size}-{y_max * pixel_size}"
         )
 
         # Check if there's a small isolated region
         # Count connected components
-        from scipy import ndimage
-
         labeled_array, num_features = ndimage.label(mask)

---

101-112: Hardcoded coordinates suggest incomplete implementation.

The script uses hardcoded coordinates (3345-3350, 2535-2540) to identify a "problematic component." This suggests the script is a work-in-progress diagnostic tool rather than a general-purpose analyzer.

Consider parameterizing these coordinates or removing this section if it was intended for one-time debugging.

tests/debug_binary.py (1)

164-164: Hardcoded filename limits reusability.

The script hardcodes map_data_20250728_194519.bin. Consider accepting the filename as a command-line argument for better flexibility.

🔎 Proposed enhancement

 def main():
     """Main function."""
-    payload_file = "map_data_20250728_194519.bin"
+    import sys
+    payload_file = sys.argv[1] if len(sys.argv) > 1 else "map_data_20250728_194519.bin"
 
     if not os.path.exists(payload_file):
-        print(f"Payload file {payload_file} not found!")
+        print(f"Usage: {sys.argv[0]} [payload_file]")
+        print(f"Payload file not found: {payload_file}")
         return

tests/analyze_segment_walls.py (1)

99-134: O(n*m) complexity may cause performance issues.

The find_adjacent_pixels function uses nested loops over all segment and wall coordinates, resulting in O(n*m) complexity. For large maps, this could be very slow.

Consider using spatial data structures (e.g., KD-tree or grid-based lookup) for better performance:

from collections import defaultdict

def find_adjacent_pixels(segment_pixels, wall_pixels):
    """Find adjacent pixels using grid-based lookup."""
    # Build grid of wall pixels
    wall_grid = set()
    for x, y, length in wall_pixels:
        for i in range(length):
            wall_grid.add((x + i, y))
    
    adjacent_pairs = []
    for x, y, length in segment_pixels:
        for i in range(length):
            sx, sy = x + i, y
            # Check 8 adjacent positions
            for dx in [-1, 0, 1]:
                for dy in [-1, 0, 1]:
                    if (sx + dx, sy + dy) in wall_grid:
                        adjacent_pairs.append(((sx, sy), (sx + dx, sy + dy)))
                        break
    
    return adjacent_pairs

tests/analyze_room_connections.py (2)

104-150: Brute-force distance calculation is inefficient.

The minimum distance calculation uses nested loops over all pixel pairs, resulting in O(n*m) complexity. For rooms with thousands of pixels, this will be extremely slow.

Consider using scipy's distance_transform_edt or cdist for much better performance:

from scipy.spatial.distance import cdist

# Calculate minimum distance between Room 2 and Room 7
if len(room2_indices[0]) > 0 and len(room7_indices[0]) > 0:
    room2_coords = np.column_stack((room2_indices[1], room2_indices[0]))
    room7_coords = np.column_stack((room7_indices[1], room7_indices[0]))
    
    # Compute pairwise distances
    distances = cdist(room2_coords, room7_coords, metric='euclidean')
    
    # Find minimum
    min_idx = np.unravel_index(distances.argmin(), distances.shape)
    min_distance = distances[min_idx]
    closest_point_room2 = tuple(room2_coords[min_idx[0]])
    closest_point_room7 = tuple(room7_coords[min_idx[1]])

---

45-79: Duplicated segment processing logic.

The segment processing code appears twice (lines 45-79 and lines 160-197) with nearly identical logic. Extract this into a reusable function to follow DRY principles.

🔎 Proposed refactoring

def process_segment(layer, height, width):
    """Process a single segment layer into a mask."""
    segment_id = layer.get("metaData", {}).get("segmentId")
    name = layer.get("metaData", {}).get("name", f"Room {segment_id}")
    
    compressed_pixels = layer.get("compressedPixels", [])
    pixels = [
        compressed_pixels[i : i + 3]
        for i in range(0, len(compressed_pixels), 3)
    ]
    
    mask = np.zeros((height, width), dtype=np.uint8)
    for pixel_run in pixels:
        x, y, length = pixel_run
        if 0 <= y < height and 0 <= x < width and x + length <= width:
            mask[y, x : x + length] = 1
    
    return segment_id, name, mask

# Use in both places
for layer in data.get("layers", []):
    if layer.get("__class") == "MapLayer" and layer.get("type") == "segment":
        segment_id, name, room_mask = process_segment(layer, height, width)
        # ... rest of logic

Also applies to: 160-197

tests/tests/test_all_bins.py (2)

189-190: Searching parent directory for test files is fragile.

The script searches for .bin files in the parent directory (..), which makes the test dependent on directory structure and could find unintended files.

Consider:

1. Using a dedicated test data directory
2. Accepting test file paths as command-line arguments
3. Using a configuration file to specify test files

# Example: Use dedicated test directory
test_dir = os.path.join(os.path.dirname(__file__), "test_data")
bin_files = [f for f in os.listdir(test_dir) if f.endswith(".bin")]

---

62-75: Handle JSON parsing for consistency.

The special case for NEW_RAND256 returning a JSON string is handled well, but consider documenting why this parser returns JSON while others return dictionaries.

Add a comment explaining the different return types:

# For new parser, result is JSON string (for serialization optimization)
# Other parsers return dict directly
if parser_name == "NEW_RAND256" and isinstance(result, str):

tests/test_rand_to_hypfer_compression.py (1)

97-98: Hardcoded segment ID limits reusability.

The script hardcodes segment ID 20. Consider accepting the segment ID as a command-line argument to test different segments.

import sys

segment_id = int(sys.argv[1]) if len(sys.argv) > 1 else 20
print(f"Testing compression on segment {segment_id}")

tests/convert_rand_to_hypfer.py (3)

11-38: Unused variable prev_y.

The variable prev_y is assigned on line 33 but never used. Consider removing it to clean up the code.

🔎 Proposed fix

 def compress_pixels(pixel_indices, image_width, image_height, image_top=0, image_left=0):
     """Convert Rand256 pixel indices to Hypfer compressed format."""
     if not pixel_indices:
         return []
     
     compressed = []
-    prev_x = prev_y = None
+    prev_x = None
     run_start_x = run_y = None
     run_length = 0
     
     for idx in pixel_indices:
         x = (idx % image_width) + image_left
         y = ((image_height - 1) - (idx // image_width)) + image_top
         
         if run_start_x is None:
             run_start_x, run_y, run_length = x, y, 1
         elif y == run_y and x == prev_x + 1:
             run_length += 1
         else:
             compressed.extend([run_start_x, run_y, run_length])
             run_start_x, run_y, run_length = x, y, 1
         
-        prev_x, prev_y = x, y
+        prev_x = x
     
     if run_start_x is not None:
         compressed.extend([run_start_x, run_y, run_length])
     
     return compressed

---

41-71: Consider incremental min/max calculation for memory efficiency.

The current implementation expands all pixels into lists just to calculate min/max, which defeats the memory benefit of compression for large datasets. Consider calculating these incrementally.

🔎 Proposed memory-efficient implementation

 def calculate_dimensions(compressed_pixels):
     """Calculate min/max/mid/avg dimensions from compressed pixels."""
     if not compressed_pixels:
         return None
     
-    x_coords = []
-    y_coords = []
-    pixel_count = 0
+    x_min = x_max = None
+    y_min = y_max = None
+    x_sum = y_sum = pixel_count = 0
     
     for i in range(0, len(compressed_pixels), 3):
         x, y, length = compressed_pixels[i], compressed_pixels[i+1], compressed_pixels[i+2]
-        for j in range(length):
-            x_coords.append(x + j)
-            y_coords.append(y)
-            pixel_count += 1
+        x_end = x + length - 1
+        
+        if x_min is None:
+            x_min, x_max = x, x_end
+            y_min = y_max = y
+        else:
+            x_min = min(x_min, x)
+            x_max = max(x_max, x_end)
+            y_min = min(y_min, y)
+            y_max = max(y_max, y)
+        
+        # For average calculation
+        for j in range(length):
+            x_sum += x + j
+            y_sum += y
+        pixel_count += length
     
     return {
         "x": {
-            "min": min(x_coords),
-            "max": max(x_coords),
-            "mid": (min(x_coords) + max(x_coords)) // 2,
-            "avg": sum(x_coords) // len(x_coords)
+            "min": x_min,
+            "max": x_max,
+            "mid": (x_min + x_max) // 2,
+            "avg": x_sum // pixel_count
         },
         "y": {
-            "min": min(y_coords),
-            "max": max(y_coords),
-            "mid": (min(y_coords) + max(y_coords)) // 2,
-            "avg": sum(y_coords) // len(y_coords)
+            "min": y_min,
+            "max": y_max,
+            "mid": (y_min + y_max) // 2,
+            "avg": y_sum // pixel_count
         },
         "pixelCount": pixel_count
     }

---

74-80: Add error handling for file operations.

The function opens the file without a try-except block. Consider adding error handling for FileNotFoundError and json.JSONDecodeError.

🔎 Proposed fix

 def convert_rand_to_hypfer(rand_json_path, output_path):
     """Convert complete Rand256 JSON to Hypfer format."""
     
-    # Load Rand256 JSON
-    with open(rand_json_path, 'r') as f:
-        rand_data = json.load(f)
+    # Load Rand256 JSON
+    try:
+        with open(rand_json_path, 'r') as f:
+            rand_data = json.load(f)
+    except FileNotFoundError:
+        raise FileNotFoundError(f"Input file not found: {rand_json_path}")
+    except json.JSONDecodeError as e:
+        raise ValueError(f"Invalid JSON in {rand_json_path}: {e}")

backups/rand25_handler_rooms.py (4)

196-206: Unused assignment immediately overwritten.

img_np_array returned from _setup_robot_and_image (line 197) is immediately overwritten by async_copy_array (line 206). The first assignment is discarded.

🔎 Proposed simplification

                 (
-                    img_np_array,
+                    _,
                     robot_position,
                     robot_position_angle,
                 ) = await self._setup_robot_and_image(
                     m_json, size_x, size_y, colors, destinations
                 )

                 # Increment frame number
                 self.frame_number += 1
                 img_np_array = await self.async_copy_array(self.img_base_layer)

---

263-267: Incomplete room element mapping implementation.

The comment indicates this is "a simplification" with a pass statement. The element map for rooms is not being populated, which may affect room-specific features.

Would you like help implementing the room pixel identification logic for the element map?

---

430-430: Unnecessary del statements.

Explicitly deleting local variables (del room, corners, robot_x, robot_y) is unnecessary in Python. These are automatically garbage collected when the function returns.

🔎 Proposed fix

                 _LOGGER.debug("%s is in %s", self.file_name, self.robot_in_room["room"])
-                del room, corners, robot_x, robot_y  # free memory.
                 return temp

---

470-476: Inconsistent element management approach.

enable_element directly calls self.drawing_config.enable_element(), while disable_element uses the manage_drawable_elements utility. Consider using consistent approaches for both methods.

🔎 Proposed fix for consistency

     def enable_element(self, element_code: DrawableElement) -> None:
         """Enable drawing of a specific element."""
-        self.drawing_config.enable_element(element_code)
+        manage_drawable_elements(self, "enable", element_code=element_code)

tests/test_floor_data.py (1)

6-27: Consider importing the actual types instead of duplicating.

The test file duplicates TrimsData and FloorData definitions. While this enables standalone execution, it risks testing behavior that differs from the actual implementation in SCR/valetudo_map_parser/config/types.py. The real TrimsData has additional methods (from_json, to_json, from_dict, clear) not present here.

Consider importing from the actual module and adding integration tests that verify the real implementation, or ensure this duplication is kept in sync:

# Option 1: Import actual types
from SCR.valetudo_map_parser.config.types import TrimsData, FloorData

# Option 2: If standalone is required, add a note about sync requirements
# WARNING: Keep in sync with SCR/valetudo_map_parser/config/types.py

tests/tests/test_robot_angles.py (2)

8-9: Path manipulation may be fragile.

The path insertion assumes a specific directory structure relative to the script. This could fail if the script is run from a different working directory. Consider using a more robust approach.

🔎 Proposed fix

 # Add the SCR directory to Python path
-sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), "SCR")))
+# Navigate from tests/tests/ to repository root, then to SCR
+sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..", "SCR")))

Note: The current path joins tests/tests/ with SCR, resulting in tests/tests/SCR which likely doesn't exist.

---

26-189: Script serves as documentation/analysis rather than automated tests.

This is an exploratory analysis script comparing angle calculation approaches, not a pytest-compatible test suite. The formatted output and recommend_solution() function that prints code suggestions make this clear.

Consider:

1. Renaming to analyze_robot_angles.py to clarify its purpose
2. Moving to a scripts/ or analysis/ directory
3. If test coverage is needed, create separate pytest-compatible tests

backups/drawable_ori.py (1)

164-179: Dead code: pole_alpha blending condition will never trigger.

pole_alpha is hardcoded to 255 on line 164, so the condition if pole_alpha < 255: on line 176 will never be true. The blending code on lines 177-179 is unreachable.

🔎 Suggested fix

         # Get blended colors for flag and pole
         flag_alpha = flag_color[3] if len(flag_color) == 4 else 255
         pole_color_base = (0, 0, 255)  # Blue for the pole
-        pole_alpha = 255
 
         # Blend flag color if needed
         if flag_alpha < 255:
             flag_color = ColorsManagement.sample_and_blend_color(
                 layer, x, y, flag_color
             )
 
         # Create pole color with alpha
-        pole_color: Color = (*pole_color_base, pole_alpha)
-
-        # Blend pole color if needed
-        if pole_alpha < 255:
-            pole_color = ColorsManagement.sample_and_blend_color(
-                layer, x, y, pole_color
-            )
+        pole_color: Color = (*pole_color_base, 255)

backups/colors.py (3)

269-276: Unnecessary use of enumerate - index is unused.

The loop uses enumerate but discards the index with _. Simply iterate over the zipped values directly.

🔎 Suggested fix

             # Process RGB colors
             result = []
-            for _, (alpha, rgb) in enumerate(zip(alphas, rgb_colors)):
+            for alpha, rgb in zip(alphas, rgb_colors):
                 if rgb is None:
                     result.append((0, 0, 0, int(alpha)))
                 else:
                     result.append((rgb[0], rgb[1], rgb[2], int(alpha)))

---

489-512: Questionable optimization: ndimage.map_coordinates overhead for single-pixel sampling.

Using ndimage.map_coordinates for sampling a single pixel (4 separate calls for each channel) adds significant overhead compared to direct indexing. The comment claims this is more efficient for large arrays, but for single-point lookups, direct indexing array[y, x] is O(1) and much faster.

Consider removing the ndimage path for single-point sampling, or document specific benchmarks that justify this approach.

---

822-824: Performance: list(SupportedColor).index() is O(n) per call.

Creating a list from the enum and calling .index() on every get_colour call is inefficient. Consider caching this mapping.

🔎 Suggested fix

# Add as class attribute in ColorsManagement.__init__:
self._color_index_map = {color: idx for idx, color in enumerate(SupportedColor) 
                         if not color.startswith(SupportedColor.COLOR_ROOM_PREFIX)}

# Then in get_colour:
try:
    index = self._color_index_map[supported_color]
    return self.user_colors[index]
except KeyError:
    ...

tests/PROFILING_README.md (1)

102-124: Add language specifiers to Python code blocks.

The Python code blocks at lines 102-111 and 114-124 should specify the language for better syntax highlighting and accessibility.

🔎 Proposed fix

Update the code fence opening at line 102:

-```
+```python

Update the code fence opening at line 114:

-```
+```python

new_tests/IMPLEMENTATION_SUMMARY.md (1)

33-54: Consider adding language specifier to directory tree.

The directory tree structure code block could specify a language (e.g., tree or text) for consistency with other code blocks in the documentation.

🔎 Proposed fix

-```
+```text
 new_tests/

new_tests/config/test_colors.py (2)

106-112: Consider strengthening the assertion for mismatched lengths.

The test only verifies that the result is a list. Based on the implementation context, it would be more valuable to assert the expected behavior - whether it returns a truncated list, pads with defaults, or raises an exception.

🔎 Suggested improvement

     def test_add_alpha_to_rgb_mismatched_lengths(self):
         """Test adding alpha to RGB colors with mismatched lengths."""
         alpha_channels = [255.0, 128.0]
         rgb_colors = [(255, 0, 0), (0, 255, 0), (0, 0, 255)]
         result = ColorsManagement.add_alpha_to_rgb(alpha_channels, rgb_colors)
         # Should handle mismatch gracefully
         assert isinstance(result, list)
+        # Verify the specific handling behavior (e.g., truncation or padding)
+        # Adjust based on actual implementation behavior
+        assert len(result) == min(len(alpha_channels), len(rgb_colors))

---

155-163: Cache emptiness assertion may be fragile.

The assertion that color_cache is empty at initialization (line 160) could break if the implementation pre-populates the cache. Consider documenting this as testing current behavior rather than a requirement, or verify this is intentional.

tests/test_room_store.py (2)

15-38: Non-standard import pattern using importlib.

This manual module loading approach is unusual for pytest tests. It manually constructs the module path and loads dependencies, which is fragile and harder to maintain.

Consider using standard pytest configuration with conftest.py to set up the Python path, or use pytest.ini/pyproject.toml to configure the test paths. This would allow standard imports like:

from valetudo_map_parser.config.types import RoomStore

🔎 Alternative using conftest.py

In tests/conftest.py:

import sys
from pathlib import Path

# Add SCR to path once at test collection time
sys.path.insert(0, str(Path(__file__).parent.parent / "SCR"))

Then in this file:

-import importlib.util
-import logging
-import sys
-from pathlib import Path
-
-# Add SCR/valetudo_map_parser to path so relative imports work
-valetudo_path = Path(__file__).parent.parent / "SCR" / "valetudo_map_parser"
-if str(valetudo_path.parent) not in sys.path:
-    sys.path.insert(0, str(valetudo_path.parent))
-
-# Load const module first
-const_path = valetudo_path / "const.py"
-const_spec = importlib.util.spec_from_file_location("valetudo_map_parser.const", const_path)
-const_module = importlib.util.module_from_spec(const_spec)
-sys.modules["valetudo_map_parser.const"] = const_module
-const_spec.loader.exec_module(const_module)
-
-# Now load types module
-types_path = valetudo_path / "config" / "types.py"
-spec = importlib.util.spec_from_file_location("valetudo_map_parser.config.types", types_path)
-types = importlib.util.module_from_spec(spec)
-sys.modules["valetudo_map_parser.config.types"] = types
-spec.loader.exec_module(types)
-
-RoomStore = types.RoomStore
+import logging
+from valetudo_map_parser.config.types import RoomStore

---

41-46: Excessive logging in test functions.

The tests use extensive _LOGGER.info() calls which will produce verbose output during test runs. While useful for debugging, this can clutter pytest output.

Consider using pytest's built-in -v flag for verbosity or capsys/caplog fixtures for capturing output, and reserve explicit logging for failure diagnostics.

Also applies to: 51-53, 134-136

tests/test_status_text_performance.py (2)

114-114: Remove redundant import.

sys is already imported at the module level (line 10). This duplicate import inside the function is unnecessary.

-    import sys
     func_list_size = sys.getsizeof(status_text.compose_functions)

---

34-184: Consider adding pytest assertions for CI integration.

This performance test uses print statements for reporting but lacks pytest assertions. For CI integration, consider adding minimum performance thresholds or using pytest-benchmark for standardized benchmarking.

At minimum, the overhead check (lines 170-173) could be converted to an assertion:

🔎 Optional: Add assertion for CI

     if abs(loop_time - inline_time) < 2:  # Within 2ms for 10k calls
         print("✅ Function loop overhead is NEGLIGIBLE!")
+        # Optionally assert for CI
+        assert True, "Overhead within acceptable range"
     else:
         print(f"⚠️  Function loop adds ~{overhead_per_call:.2f} μs per call")
+        # Consider: assert overhead_per_call < MAX_ACCEPTABLE_OVERHEAD_US

tests/compare_payloads.py (2)

95-105: Hardcoded test data paths may not exist.

The payload file paths are hardcoded and reference specific test data files. The function handles missing files gracefully (lines 22-24), but consider adding these files to .gitignore if they contain sensitive data, or documenting their expected location/format.

---

51-75: Consider documenting magic numbers.

The header offset 0x14 (line 52) and block type 8 (line 61) are magic numbers. For maintainability, consider adding comments or using named constants to document their meaning.

-    # Find robot position block
-    offset = 0x14  # Start after header
+    # Find robot position block
+    HEADER_SIZE = 0x14  # 20 bytes: standard RRM header size
+    ROBOT_POSITION_BLOCK_TYPE = 8
+    offset = HEADER_SIZE
     robot_block_data = None

     while offset < len(payload) - 8:
         try:
             type_ = struct.unpack("<H", payload[offset : offset + 2])[0]
             hlength = struct.unpack("<H", payload[offset + 2 : offset + 4])[0]
             length = struct.unpack("<I", payload[offset + 4 : offset + 8])[0]

-            if type_ == 8:  # Robot position block
+            if type_ == ROBOT_POSITION_BLOCK_TYPE:

tests/test_mvcrender.py (1)

10-22: Unnecessary semicolons and minor style issues.

Lines 14, 15, 18, and 20 have trailing semicolons which are not idiomatic Python. While syntactically valid, they should be removed for consistency.

🔎 Proposed fix

     def __init__(self):
         self.trims = type("T", (),
                           {"to_dict": lambda self: {"trim_up": 0, "trim_down": 0, "trim_left": 0, "trim_right": 0}})()
-        self.offset_top = 0;
-        self.offset_down = 0;
+        self.offset_top = 0
+        self.offset_down = 0
         self.offset_left = 0;
         self.offset_right = 0
-        self.vacuum_state = "cleaning";
+        self.vacuum_state = "cleaning"
         self.image_auto_zoom = True
-        self.image_ref_width = 0;
+        self.image_ref_width = 0
         self.image_ref_height = 0

tests/benchmark_margins.py (2)

39-49: Async functions contain no await statements.

Both original_image_margins and optimized_image_margins are declared as async but contain no await expressions. This adds overhead without benefit.

🔎 Proposed fix

 # Original implementation for comparison
-async def original_image_margins(
+def original_image_margins(
     image_array: np.ndarray, detect_colour: tuple
 ) -> tuple[int, int, int, int]:
     """Original implementation of the image margins function"""
     nonzero_coords = np.column_stack(np.where(image_array != list(detect_colour)))
     # Calculate the trim box based on the first and last occurrences
     min_y, min_x, _ = np.min(nonzero_coords, axis=0)
     max_y, max_x, _ = np.max(nonzero_coords, axis=0)
     del nonzero_coords
     return min_y, min_x, max_x, max_y

And similarly for optimized_image_margins. Then update the benchmark calls to remove await.

---

99-101: Unused AutoCrop instance.

The auto_crop instance is created but never used in the benchmark. Either remove it or document why it's needed for setup.

🔎 Proposed fix

         # Create AutoCrop instance
         handler = DummyHandler()
-        auto_crop = AutoCrop(handler)
+        # auto_crop = AutoCrop(handler)  # Not used in margin benchmarks

tests/test_parser_comparison.py (1)

194-210: Redundant directory existence check.

os.path.exists(payload_dir) where payload_dir = "." will always return True since the current directory always exists. This check is effectively dead code.

🔎 Proposed fix

 def main():
     """Main profiling function."""
     payload_dir = "."
     runs = 5  # Number of runs for profiling

-    if not os.path.exists(payload_dir):
-        print(f"Payload directory {payload_dir} doesn't exist.")
-        print("Run your vacuum first to generate payload files.")
-        return
-
     # Find all payload files
     payload_files = [f for f in os.listdir(payload_dir) if f.endswith(".bin")]

     if not payload_files:

Or, if you want to support a configurable directory in the future, parameterize payload_dir.

new_tests/conftest.py (1)

145-151: Consider using scope="session" for the event loop fixture.

The event_loop fixture creates a new loop for each test. If tests are independent, this is fine, but consider scope="session" or scope="module" for better performance if tests can share a loop.

tests/test_hypfer_profiling.py (1)

254-257: Unconventional function naming __main__.

The function is named __main__ which is unusual (typically main). While it works, the double underscores suggest a dunder method which this is not. Consider renaming to main().

🔎 Proposed fix

-def __main__():
+def main():
     # Enable comprehensive profiling
     test = TestHypferImageHandler(enable_profiling=True)
     ...

 if __name__ == "__main__":
-    __main__()
+    main()

tests/rand_rooms_test.py (3)

125-197: Dead code: _process_segment_data method is never called.

The _process_segment_data method is defined but never used. The async_extract_room_properties method duplicates its logic instead of calling it. Either remove this method or refactor to use it.

🔎 Proposed fix

Refactor async_extract_room_properties to use _process_segment_data:

         # Process each segment
         if unsorted_id and self.segment_data and self.outlines:
             for idx, segment_id in enumerate(unsorted_id):
-                # Extract points from segment data
-                points = []
-                for x, y, _ in self.segment_data[idx]:
-                    points.append((int(x), int(y)))
-
-                if not points:
-                    continue
-
-                # Use convex hull to get the outline
-                ... (duplicate logic)
-
-                room_properties[room_id] = room_data
+                room_id, room_data = await self._process_segment_data(
+                    self.segment_data[idx], segment_id, pixel_size
+                )
+                if room_id and room_data:
+                    # Update room name from destinations if available
+                    if segment_id in room_id_to_data:
+                        room_info = room_id_to_data[segment_id]
+                        room_data["name"] = room_info.get("name", room_data["name"])
+                    room_properties[room_id] = room_data

---

230-278: Variable shadowing: room_data used for different purposes.

room_data is first assigned on line 232 as a list from dest_json.get("rooms", []), then reassigned on line 265 as a dictionary for each room's properties. This shadowing reduces readability.

🔎 Proposed fix

         # Process destinations JSON to get room names
         dest_json = destinations
-        room_data = dest_json.get("rooms", [])
-        room_id_to_data = {room["id"]: room for room in room_data}
+        dest_rooms = dest_json.get("rooms", [])
+        room_id_to_data = {room["id"]: room for room in dest_rooms}

         # Process each segment
         ...

---

108-123: Broad exception handling masks errors.

Catching all exceptions with except Exception as e in convex_hull_outline could hide bugs. Consider catching specific exceptions like scipy.spatial.QhullError.

🔎 Proposed fix

+from scipy.spatial import ConvexHull, QhullError
+
         try:
             # Calculate the convex hull
             hull = ConvexHull(points_array)
             ...
-        except Exception as e:
+        except (QhullError, ValueError) as e:
             _LOGGER.warning(f"Error calculating convex hull: {e}")

tests/rooms_test.py (2)

252-265: Fragile pattern using "mask_offset" in locals().

Checking for variables in locals() is fragile and error-prone. Consider restructuring to always define mask_offset (as None when not applicable) and check for None instead.

🔎 Proposed fix

+            mask_offset = None  # Initialize before the conditional
             # Optimization: Create a smaller mask for just the room area
             if not pixels:
                 # Skip if no pixels
                 mask = np.zeros((1, 1), dtype=np.uint8)
             else:
                 ...
-                        # Store the offset for later use when converting coordinates back
-                        mask_offset = (min_x, min_y)
+                        mask_offset = (min_x, min_y)
                 else:
                     mask = np.zeros((1, 1), dtype=np.uint8)

             ...

             # Extract contour from the mask
-            if "mask_offset" in locals():
+            if mask_offset is not None:
                 # If we're using a local mask, we need to adjust the coordinates
                 outline = convex_hull_outline(mask)
                 if outline:
                     # Adjust coordinates back to global space
                     offset_x, offset_y = mask_offset
                     outline = [(x + offset_x, y + offset_y) for (x, y) in outline]
-                    # Clear the mask_offset variable for the next iteration
-                    del mask_offset
+                    mask_offset = None  # Reset for next iteration

---

274-277: Redundant list comprehension.

Line 276 creates a new list with the same content: [(x, y) for (x, y) in outline]. This is a no-op and can be removed.

🔎 Proposed fix

-            # Use coordinates as-is without flipping Y coordinates
-            # This prevents the large Y values caused by height - 1 - y transformation
-            outline = [(x, y) for (x, y) in outline]
+            # Note: Using coordinates as-is without flipping Y coordinates
+            # This prevents the large Y values caused by height - 1 - y transformation

backups/hypfer_rooms_handler.py (1)

314-349: Method is marked async but contains no await expressions.

get_room_at_position doesn't perform any async operations. This adds unnecessary overhead from creating coroutines and requires callers to await unnecessarily. Consider making it a regular synchronous method.

🔎 Suggested fix

-    async def get_room_at_position(
+    def get_room_at_position(
         self, x: int, y: int, room_properties: Optional[RoomsProperties] = None
     ) -> Optional[Dict[str, Any]]:

backups/hypfer_handler_ori.py (1)

444-456: Inconsistent element enable/disable implementation.

enable_element directly calls self.drawing_config.enable_element() with extra logging, while disable_element uses the manage_drawable_elements helper. This inconsistency could lead to different behavior and makes the code harder to maintain.

🔎 Suggested fix

     def enable_element(self, element_code: DrawableElement) -> None:
         """Enable drawing of a specific element."""
-        self.drawing_config.enable_element(element_code)
-        LOGGER.info(
-            "%s: Enabled element %s, now enabled: %s",
-            self.file_name,
-            element_code.name,
-            self.drawing_config.is_enabled(element_code),
-        )
+        manage_drawable_elements(self, "enable", element_code=element_code)
 
     def disable_element(self, element_code: DrawableElement) -> None:
         """Disable drawing of a specific element."""
         manage_drawable_elements(self, "disable", element_code=element_code)

backups/test_old_pars.py (2)

53-107: Recursive parsing may cause stack overflow on large maps.

parse_block uses tail recursion to process blocks sequentially. For maps with many blocks, this could exceed Python's default recursion limit (~1000). Consider converting to an iterative approach.

🔎 Suggested iterative approach

     @staticmethod
     def parse_block(
         buf: bytes,
         offset: int,
         result: Optional[Dict[int, Any]] = None,
         pixels: bool = False,
     ) -> Dict[int, Any]:
         """Parse a block of data from the map data."""
         result = result or {}
-        if len(buf) <= offset:
-            return result
-
-        type_ = struct.unpack("<H", buf[offset : offset + 2])[0]
-        hlength = struct.unpack("<H", buf[offset + 2 : offset + 4])[0]
-        length = struct.unpack("<I", buf[offset + 4 : offset + 8])[0]
-        # ... block parsing logic ...
-        return RRMapParser.parse_block(buf, offset + length + hlength, result, pixels)
+        while offset < len(buf):
+            type_ = struct.unpack("<H", buf[offset : offset + 2])[0]
+            hlength = struct.unpack("<H", buf[offset + 2 : offset + 4])[0]
+            length = struct.unpack("<I", buf[offset + 4 : offset + 8])[0]
+            # ... block parsing logic (same as current) ...
+            offset = offset + length + hlength
+        return result

---

323-334: Unreachable exception handler - dict.get() never raises KeyError.

The try/except KeyError block is dead code since dict.get() returns None (or default) instead of raising KeyError when a key is missing.

🔎 Suggested fix

     @staticmethod
     def get_rrm_goto_target(json_data: JsonType) -> list or None:
         """Get the goto target from the json."""
-        try:
-            path_data = json_data.get("goto_target", {})
-        except KeyError:
-            return None
-
+        path_data = json_data.get("goto_target")
-        if path_data and path_data != []:
+        if path_data:
             path_data = RandImageData.rrm_coordinates_to_valetudo(path_data)
             return path_data
         return None

The same issue exists in get_rrm_virtual_walls (Lines 401-409).

backups/map_data_ori.py (1)

40-43: Unnecessary try/except - dict.get() doesn't raise KeyError.

🔎 Suggested fix

     @staticmethod
     def get_obstacles(entity_dict: dict) -> list:
         """Get the obstacles positions from the entity data."""
-        try:
-            obstacle_data = entity_dict.get("obstacle")
-        except KeyError:
-            return []
+        obstacle_data = entity_dict.get("obstacle")
         obstacle_positions = []

backups/drawable.ori (1)

906-908: Potential AttributeError when path_font is empty string.

Line 908 calls path_font.endswith("VT.ttf"), but if path_font is an empty string (which passes the if path_font: check on Line 892), the method call works but Line 906-907 would have already set font = user_font. However, the logic is convoluted - if path_font is provided but fails to load, user_font equals default_font, making the endswith check pointless in that case.

🔎 Suggested fix for clarity

             if "\u2211" in text or "\u03de" in text:
                 font = default_font
                 width = None
             else:
                 font = user_font
-                width = 2 if path_font.endswith("VT.ttf") else None
+                width = 2 if path_font and path_font.endswith("VT.ttf") else None